-
Notifications
You must be signed in to change notification settings - Fork 12
Perfetto tracing #286
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Perfetto tracing #286
Conversation
TwitchBronBron
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see much value in having two separate broken-up PRs for this functionality, so let's just close #285 in favor of this one. I've left several comments. Thanks for working on this!
package.json
Outdated
| "fs-extra": "^10.0.0", | ||
| "glob": "^7.2.0", | ||
| "natural-orderby": "^2.0.3", | ||
| "path": "^0.12.7", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
path is already part of nodejs, so let's remove this as a dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/PerfettoController.ts
Outdated
| private stopCB: (() => void) | null = null; // To store the stop callback | ||
| private selectedChannel: string = 'dev'; | ||
|
|
||
| public async startTracing(fileUriPath: string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not loving the if useWebsocket else logic througout this whole file. It makes each function more complex to read. It might be better to have two separate classes, something like PerfettoWebsocketClient and PerfettoEcpClient. They'd have a common interface with methods like enable, start, stop, saveToFile, or whatever else you'd need. Then your logic in here could be much simpler. You'd instantiate the correct one when we start, and then just to this.client.doTheTHing() in these methods below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
created a single class "PerfettoManager"
support to connect WS and save the trace to a file